gate: fix script context leakage and empty envelope parsing#56
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bd755ba0cc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if err := json.Unmarshal(rawItems, &wrapped.Items); err != nil { | ||
| return nil, fmt.Errorf("parse wrkr inventory: %w", err) | ||
| } | ||
| entries = wrapped.Items |
There was a problem hiding this comment.
Validate
items is an array in Wrkr envelope
This branch now accepts {"items":null} because unmarshalling null into a slice succeeds and yields nil, so malformed inventory payloads are treated as an empty inventory instead of an error. In cmd/gait/gate.go, high-risk/oss-prod fail-closed behavior for --wrkr-inventory is only triggered when LoadWrkrInventory returns an error, so this allows a broken inventory file to silently disable wrkr.* context enrichment and can cause context-based block rules to be skipped.
Useful? React with 👍 / 👎.
| if err := json.Unmarshal(rawEntries, &envelope.Entries); err != nil { | ||
| return nil, fmt.Errorf("parse approved script registry: %w", err) | ||
| } | ||
| return normalizeApprovedScriptEntries(envelope.Entries) |
There was a problem hiding this comment.
Validate
entries is an array in registry envelope
This parsing path also treats {"entries":null} as valid because unmarshalling null into a slice returns no error, so invalid registry content is silently normalized to an empty set. cmd/gait/gate.go only enters its fail-closed path for configured approved-script registries when ReadApprovedScriptRegistry returns an error, so malformed registry state no longer surfaces as an unavailable/invalid registry in high-risk or oss-prod contexts.
Useful? React with 👍 / 👎.
Problem
Recent review findings identified three correctness gaps in Gate:
wrkr.*context across steps due to sharedauth_contextmap references.{"entries":[]}).{"items":[]}).These can produce incorrect policy matches and fail-closed blocks for syntactically valid configs.
Changes
core/gate/policy.goApplyWrkrContextin script evaluation to isolate step-local context mutations.core/gate/approved_scripts.goentrieskey is present, including empty arrays.core/gate/context_wrkr.goitemskey is present, including empty arrays.core/gate/policy_test.go: no Wrkr context leakage across script steps.core/gate/approved_scripts_test.go: empty registry envelope parses as empty set.core/gate/context_wrkr_test.go: empty Wrkr envelope parses as empty set.Validation
make prepush-full./gait doctor --jsongo test ./core/gatego test ./cmd/gait -run "GateEval|ApproveScript|ListScripts"bash scripts/run_scenarios.sh gaitbash scripts/test_script_intent_acceptance.sh ./gait